Review — PR #209

Verdict: approve ✅

Summary

This PR correctly fixes the two documentation bugs described in #190: the orphaned elif in the pre-script example and the missing issues: write permission. It goes beyond the minimal fix by removing the entire Jira code path rather than just converting elif to if, which is the right approach since Jira support was already removed from the guide's examples upstream (PR fullsend-ai#2568). The cleanup is thorough — all references to issue_source/ISSUE_SOURCE and the Jira credential variables within the guide are addressed consistently across the pre-script, workflow YAML, trigger docs, and slash-command dispatch examples.

Findings

Dimensions Reviewed

• Correctness ✅ — Pre-script example is syntactically valid bash after the change. Workflow YAML has exactly one permissions: block with all three required permissions. No stale references to removed identifiers outside the changed file (other uses of ISSUE_SOURCE in the repo are working qualityflow agents, not leftovers from this guide).
• Security ✅ — Jira credential-handling pattern removed from the example (positive change). No secrets, injection, or steganography patterns detected. The issues: write permission is authorized by Fix elif syntax error and duplicate permissions block in building-custom-agents guide #190 and consistent with the guide's post-script pattern, though the demonstrated operations only require read access.
• Intent & coherence ✅ — All changes trace to issue Fix elif syntax error and duplicate permissions block in building-custom-agents guide #190's authorization. Removing the full Jira code path (rather than just fixing elif → if) is within scope per the issue's optional cleanup clause and is the correct approach since Jira support was already removed from these examples upstream.
• Style & conventions ✅ — Formatting, indentation, and permission ordering are consistent with the rest of the file and sibling documents.
• Documentation currency ✅ — No stale references to removed identifiers in other documentation files. The remaining Jira mention at line 223 ("Never allow Jira/internal APIs from the sandbox") is a general security design principle applicable to any agent, not a reference to the removed example code.

Review

Findings

Low

• [permission-expansion] docs/guides/user/building-custom-agents.md:345 — The diff adds issues: write to the main agent workflow example (Step 7), but the pre-script only uses gh issue view (a read operation) and the example post-script does not write to issues. At most issues: read is needed for the example as written. Granting issues: write in example code that users will copy violates least-privilege.
  Remediation: Replace issues: write with issues: read in the main workflow example, since only gh issue view is used. Add an inline comment noting that users should upgrade to issues: write if their post-script creates or updates issues.

Labels: PR fixes documentation bugs in user guide

Review

Findings

Medium

• [edge-case] docs/guides/user/building-custom-agents.md:265 — The pre-script example references $REPO_FULL_NAME, but this variable is never defined in the workflow env block (Step 7) or in the harness runner_env (Step 2). With set -euo pipefail, the script will fail immediately with an unbound variable error. This was pre-existing (the old elif branch had the same issue), but the PR promotes it from a conditional branch to unconditional code, making it a guaranteed failure for anyone who copies the example verbatim.
  Remediation: Replace "$REPO_FULL_NAME" with "$GITHUB_REPOSITORY" (automatically available in GitHub Actions), or add REPO_FULL_NAME: ${{ github.repository }} to the workflow env block.

Low

• [stale-reference] docs/guides/user/building-custom-agents.md:223 — The policy design principles section still references Jira ("Never allow Jira/internal APIs from the sandbox...") while the PR removes all Jira examples from the guide. Consider generalizing to "Never allow external issue-tracking APIs from the sandbox. All external reads happen in pre-scripts; all writes happen in post-scripts."
• [permission-expansion] docs/guides/user/building-custom-agents.md:345 — issues: write is added to the main workflow template's permissions block, but the example workflow contains no code that writes to issues. The GH_TOKEN with this permission flows through the harness and potentially into the sandbox. The slash-command dispatch workflow later in the guide already correctly scopes issues: write to just the dispatch job. Consider removing it from the main workflow or adding a comment explaining when it is needed.